PVS and Clang-tidy modify #1077
Conversation
dota17
requested review from
baylesj,
BillyDonahue and
cdunn2001
and removed request for
baylesj and
BillyDonahue
|
These are all good changes, but I'd really rather see the test-coverage go up before we worry about cosmetic improvements. |
Hi @cdunn2001 Since the latest version is only a pre-release version, and some users feedback that there is a little problem with it, so will we need to release a stable version after the coverge reach the goal? |
dota17
force-pushed
the
PVSandClangtidyModify
branch
from
c449ba6 to
3935acd
Compare
| @@ -56,7 +56,7 @@ static Json::String readInputTestFile(const char* path) { | |||
| return ""; | |||
| fseek(file, 0, SEEK_END); | |||
| long const size = ftell(file); | |||
| size_t const usize = static_cast<unsigned long>(size); | |||
| const auto usize = static_cast<size_t>(size); | |||
There was a problem hiding this comment.
All this buggy fuss.
Meanwhile this whole function is conceptually 4 easy lines. :)
static Json::String readInputTestFile(const char* path) {
std::ifstream in(path, in.binary);
std::stringstream os;
os << in.rdbuf();
return Json::String(os.str());
}
There was a problem hiding this comment.
Yes, that's great! but this sould be in another PR.
| int delta = int(value_.map_->size() - other.value_.map_->size()); | ||
| if (delta) | ||
| return delta < 0; | ||
| auto thisSize = value_.map_->size(); |
There was a problem hiding this comment.
We could try doing it an easier way.
// order by map size, then by map contents.
auto tieFields = [](const Value& v) { return std::tie(v.map_->size(), *v.map_); };
return tieFields(*this) < tieFields(other);
Well, I thought someone said the jsontestrunner used the old Reader, not the new CharReader. If that's true, we are missing a lot of behavioral coverage. But maybe I misunderstood that. |
Fix some remaining lower priority warnings in issue #747:
Because that PVS Studio report is old, I use clang-tidy tools to scan jsoncpp recently like #1033 #1048 and find two warnings: